<html>
<head><meta charset="utf-8"><title>Direction for Substitute Logical golds to any() or all() · clippy · Zulip Chat Archive</title></head>
<h2>Stream: <a href="https://rust-lang.github.io/zulip_archive/stream/257328-clippy/index.html">clippy</a></h2>
<h3>Topic: <a href="https://rust-lang.github.io/zulip_archive/stream/257328-clippy/topic/Direction.20for.20Substitute.20Logical.20golds.20to.20any().20or.20all().html">Direction for Substitute Logical golds to any() or all()</a></h3>

<hr>

<base href="https://rust-lang.zulipchat.com">

<head><link href="https://rust-lang.github.io/zulip_archive/style.css" rel="stylesheet"></head>

<a name="234428387"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/257328-clippy/topic/Direction%20for%20Substitute%20Logical%20golds%20to%20any%28%29%20or%20all%28%29/near/234428387" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Arya Kumar <a href="https://rust-lang.github.io/zulip_archive/stream/257328-clippy/topic/Direction.20for.20Substitute.20Logical.20golds.20to.20any().20or.20all().html#234428387">(Apr 14 2021 at 01:39)</a>:</h4>
<p>Hi all! I'd like to try my hand at the new lint recommended for <a href="https://github.com/rust-lang/rust-clippy/issues/6827">https://github.com/rust-lang/rust-clippy/issues/6827</a>. </p>
<p>Specifically, it was noted that <code>unnecessary_fold</code> did not catch <code>&amp;</code> and <code>|</code> bitwise operators. <span class="user-mention" data-user-id="360405">@Cameron Steffen</span> had mentioned this should be a new lint. I want to try and tackle this over the next few days (hopefully this weekend). Would you mind elaborating slightly on your expectations for the new lint, <code>bitwise_bool</code>? Specifically, is this just copying <code>unnecessary_fold</code> and replacing logical operators with bitwise ones? What other changes would you want me to make?</p>



<a name="234432036"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/257328-clippy/topic/Direction%20for%20Substitute%20Logical%20golds%20to%20any%28%29%20or%20all%28%29/near/234432036" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Cameron Steffen <a href="https://rust-lang.github.io/zulip_archive/stream/257328-clippy/topic/Direction.20for.20Substitute.20Logical.20golds.20to.20any().20or.20all().html#234432036">(Apr 14 2021 at 02:31)</a>:</h4>
<p>It should simply detect <code>a &amp; b</code> that could be <code>a &amp;&amp; b</code>. Same with <code>|</code>. That's all. No need to predict what lints would be triggered after <code>bitwise_bool</code> is fixed.</p>



<a name="234505956"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/257328-clippy/topic/Direction%20for%20Substitute%20Logical%20golds%20to%20any%28%29%20or%20all%28%29/near/234505956" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Arya Kumar <a href="https://rust-lang.github.io/zulip_archive/stream/257328-clippy/topic/Direction.20for.20Substitute.20Logical.20golds.20to.20any().20or.20all().html#234505956">(Apr 14 2021 at 14:17)</a>:</h4>
<p>Oh I see, that’s much simpler than what I was expecting. Thanks for the clarification :)</p>



<a name="236100153"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/257328-clippy/topic/Direction%20for%20Substitute%20Logical%20golds%20to%20any%28%29%20or%20all%28%29/near/236100153" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Arya Kumar <a href="https://rust-lang.github.io/zulip_archive/stream/257328-clippy/topic/Direction.20for.20Substitute.20Logical.20golds.20to.20any().20or.20all().html#236100153">(Apr 25 2021 at 22:57)</a>:</h4>
<p>Hi <span class="user-mention" data-user-id="360405">@Cameron Steffen</span> I think I've written my first attempt at this lint: would you mind taking a look at it and letting me know if there's something I should change? It's PR <a href="https://github.com/rust-lang/rust/issues/7133">#7133</a></p>



<a name="236103507"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/257328-clippy/topic/Direction%20for%20Substitute%20Logical%20golds%20to%20any%28%29%20or%20all%28%29/near/236103507" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Cameron Steffen <a href="https://rust-lang.github.io/zulip_archive/stream/257328-clippy/topic/Direction.20for.20Substitute.20Logical.20golds.20to.20any().20or.20all().html#236103507">(Apr 25 2021 at 23:54)</a>:</h4>
<p>Looks like you got the right idea! I was not aware of the short-circuiting difference. I think you should avoid linting if <code>rhs_operand.can_have_side_effects()</code> (on <code>Expr</code>). Also <code>style</code> would be a better category, if not pedantic.</p>



<a name="238350379"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/257328-clippy/topic/Direction%20for%20Substitute%20Logical%20golds%20to%20any%28%29%20or%20all%28%29/near/238350379" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Arya Kumar <a href="https://rust-lang.github.io/zulip_archive/stream/257328-clippy/topic/Direction.20for.20Substitute.20Logical.20golds.20to.20any().20or.20all().html#238350379">(May 11 2021 at 17:24)</a>:</h4>
<p>Hi <span class="user-mention" data-user-id="360405">@Cameron Steffen</span> and <span class="user-mention" data-user-id="248599">@llogiq</span>! Now that my semester has ended I'm revisiting the <code>bitwise_bool -&gt; needless_bitwise_bool</code> lint <a href="https://github.com/rust-lang/rust-clippy/pull/7133">(link)</a> and I think the decision we settled on (see <a href="https://github.com/rust-lang/rust-clippy/pull/7133#issuecomment-830246268">here</a>) may require a revision (I think the logic is too restrictive).</p>
<p>Specifically, following Cameron's suggestion to use <code>right_side.can_have_side_effects()</code> is a big restriction (based on the code <a href="https://doc.rust-lang.org/stable/nightly-rustc/src/rustc_hir/hir.rs.html#1576-1631">here</a>. One example is that const functions and BinOps are always marked as having side effects.</p>
<p>For example, under the proposed logic the lint would match on <code>x &amp; !y</code> since <code>!y</code> is a UnOp with no side effects, but would not match on <code>y &amp; const_returns_true()</code>. I believe that the opposite behavior would be desired, but I'm not quite sure how to craft this / I don't have a clear set of examples of cases we would want to lint on.</p>



<a name="238362279"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/257328-clippy/topic/Direction%20for%20Substitute%20Logical%20golds%20to%20any%28%29%20or%20all%28%29/near/238362279" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Cameron Steffen <a href="https://rust-lang.github.io/zulip_archive/stream/257328-clippy/topic/Direction.20for.20Substitute.20Logical.20golds.20to.20any().20or.20all().html#238362279">(May 11 2021 at 18:43)</a>:</h4>
<p><code>can_have_side_effects</code> may not be perfect. But any flaws it has can be fixed over time and multiple lints will benefit. So I think it's okay to lean on that method. But if you want to go further, we could introduce <code>clippy_utils::expr_can_have_side_effects(cx, expr)</code> which can be smarter. For example, we can type check <code>!x</code> and know there are no side effects if <code>x</code> is <code>bool</code>.</p>



<a name="238363090"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/257328-clippy/topic/Direction%20for%20Substitute%20Logical%20golds%20to%20any%28%29%20or%20all%28%29/near/238363090" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Arya Kumar <a href="https://rust-lang.github.io/zulip_archive/stream/257328-clippy/topic/Direction.20for.20Substitute.20Logical.20golds.20to.20any().20or.20all().html#238363090">(May 11 2021 at 18:49)</a>:</h4>
<p>That makes sense, I can go ahead and lean on it. With <code>can_have_side_effects</code> in its current form, it does already mark <code>!x</code> as having no side effects. I'm having a little trouble coming up with good test cases, but I suppose they can be improved as and when <code>can_have_side_effects</code> is improved.</p>
<p>I don't feel quite equipped to handle a custom <code>clippy_utils::expr_can_have_side_effects(cx, expr)</code> just yet- I'm still pretty new to rust, and this project :)</p>



<hr><p>Last updated: Aug 07 2021 at 22:04 UTC</p>
</html>